Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warning fixes (everywhere) #766

Merged
merged 26 commits into from
Aug 8, 2022
Merged

Conversation

Daft-Freak
Copy link
Collaborator

I made the mistake of looking at my build output.

After reducing the warning count a lot, this also sets the same warning flags as the boilerplate to hopefully catch more warnings. (May result in more commits if actions reports loads)

@Daft-Freak
Copy link
Collaborator Author

... or the actions could just... not run. Warnings must be bad then.

@Daft-Freak Daft-Freak force-pushed the mostly-pico-warnings branch 3 times, most recently from b4e51ed to d12e85e Compare March 28, 2022 14:23
@Daft-Freak
Copy link
Collaborator Author

5x the commits later... macOS and MinGW are clean, STM32 has 6 warnings, PicoSystem is mostly fixed by #764, VS is... VS and I'm leaving the clang-tidy stuff in the Linux build for now.

(I ran clang-tidy and now I have a... kiloproblem?)
kiloproblem

@Daft-Freak Daft-Freak changed the title Warning fixes (mostly pico) Warning fixes (everywhere) Mar 28, 2022
@Daft-Freak
Copy link
Collaborator Author

(I ran clang-tidy and now I have a... kiloproblem?)

Hmm, it's not that bad, Daft-Freak@d7bd600 fixes more than half of those (I was bored)

@Gadgetoid
Copy link
Contributor

I guess I should merge this first and then duck under the maelstrom of merge conflicts 👀

@Daft-Freak
Copy link
Collaborator Author

Mmm yeah, I really got carried away there didn't I? 😆

This could only be a problem if reading >2GB was possible
Okay, actions won't see these but...
One of these has a missng break... but looking closer, none of them are actually used
This is causing warnings now, and doesn't appear to help any more either
This now has the opposite effect (more warnings), fun.

This reverts commit a92b87d.
None of these have custom destructors or assignment operators, so we can just use the implicitly defined copies.

This also avoids some -Wdeprecated-copy warnings
It's a 3rd-party lib and this also stops clang-tidy checking it
My local clang-tidy excludes minimp3 without this, the one in the action doesn't...
Remove unused variables and outdated commented out code
This still has the problem that all the code using ParticleGenerator is dead
@Gadgetoid Gadgetoid merged commit 516dbbd into 32blit:master Aug 8, 2022
@Gadgetoid
Copy link
Contributor

QUICK! MERGE 😆

@Daft-Freak Daft-Freak deleted the mostly-pico-warnings branch August 8, 2022 11:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants